compiler: Enhance detect_accesses and patch symbolic padding#2886
compiler: Enhance detect_accesses and patch symbolic padding#2886FabioLuporini merged 2 commits intomainfrom
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2886 +/- ##
==========================================
+ Coverage 83.43% 83.47% +0.03%
==========================================
Files 248 248
Lines 51381 51471 +90
Branches 4431 4433 +2
==========================================
+ Hits 42871 42964 +93
+ Misses 7757 7756 -1
+ Partials 753 751 -2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| # Search among the Indexeds (Most accesses typically stem from Indexeds) | ||
| plain_indexeds = retrieve_indexed(exprs, deep=True) | ||
|
|
||
| # Search among higher order objects, which still represent meaningful accesses |
There was a problem hiding this comment.
This look very specific and seems like that's something retrieve_index should catch
There was a problem hiding this comment.
the idea is that other objects can end up there, in the future, maybe...
as for retrieve_indexed catching it: disagree, it's not an implicit Indexed, it's rarther a logical representation of the base address of the TensorMove -- as an Indexed, for homogeneity
FabioLuporini
changed the title
FabioLuporini
changed the title
FabioLuporini
force-pushed
the
tma-write
branch
from
63b23b5 to
c975ff5
Compare
| value = sympify(value) | ||
| step = sympify(step) | ||
|
|
||
| if step == 0: |
There was a problem hiding this comment.
Would if step < 1 be safer? Prevents a negative number being supplied
There was a problem hiding this comment.
fair, yes, I will add that. Will also ensure it's an integer
| # Already a multiple of `mmts`, no need to pad | ||
| return nopadding | ||
| else: | ||
| from devito.symbolics import RoundUp # noqa |
There was a problem hiding this comment.
Maybe implies that extended_sympy should be moved somewhere else?
There was a problem hiding this comment.
yes, it's a long standing issue, documented somewhere
| u = Function(name='u', grid=grid) | ||
| a = dSymbol('a', dtype=np.int32) | ||
|
|
||
| expr = RoundUp(a, 16) |
There was a problem hiding this comment.
Can the round up factor also be symbolic?
There was a problem hiding this comment.
not sure, I cooked up something simple for my needs after days of frustration
There was a problem hiding this comment.
see comment above -- now ensuring it's an integer number
FabioLuporini
force-pushed
the
tma-write
branch
from
c975ff5 to
fc4c548
Compare
| a = dSymbol('a', dtype=np.int32) | ||
|
|
||
| expr = RoundUp(a, 16) | ||
| with switchconfig(platform='bdw', language='openmp'): |
There was a problem hiding this comment.
stupid AI, will patch in the next PR
|
|
||
| if step < 1: | ||
| raise ValueError("Cannot round up with negative `step`") | ||
| if not is_integer(step): |
There was a problem hiding this comment.
I would check it's an integer before checking it's greater than zero, but whatever
3 participants
Test in PRO in the PR/branch with same name as this one